-
Notifications
You must be signed in to change notification settings - Fork 370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set cookie issue 438 #449
Set cookie issue 438 #449
Conversation
fix for issue 'splunklib expects any Set-Cookie to be an auth cookie from Splunk. This is a problem when authenticating with a bearer token.'
fix for issue 'splunklib expects any Set-Cookie to be an auth cookie from Splunk. This is a problem when authenticating with a bearer token.'
This fix now ensures that cookies only will be used when there is an auth cookie |
Hi @bendikro - the fact that any/all cookies were being sent if detected was an unintentional side-effect of the bug that was addressed in this change. By design the SDK should not include cookies unrelated to Splunk - although any additional headers including cookies can be set by users to be sent by the SDK client here: splunk-sdk-python/splunklib/binding.py Lines 454 to 455 in 19acb9a
|
Hi @fantavlik What is the rationale for not including cookies in the requests when a Set-Cookie has been included in a response? What is gained by ignoring these? With "cookies unrelated to Splunk" you mean Set-Cookies that are not set by the Splunk server? The official Splunk docs even has a section on Use a load balancer with search head clustering:
From this we find:
The F5 doc has a page HTTP Load Balancing with Cookie Persistence, where "Cookie Persistence" is the "sticky" or "persistent" sessions referenced in the Splunk docs above. The solution to include additional headers manually in the client is how we worked around the issue in the first place, but it shouldn't be necessary to manually extract the BIGip cookie (after initial requests to Splunk) and then create a new client with this cookie as an additional header. It is the responsibility of the Splunk SDK client to handle this. If this behavior of the SDK truly is by design, I think the design should be reconsidered. |
@fantavlik Thanks! That fix has a bug though (#438 (comment)) but after resolving that it seems to be working properly. |
@fantavlik and @bendikro The latest version of the SOAR Splunk app is impacted by this issue also. I would suggest that the next release of the Splunk Enterprise Python SDK features list should mention support of Load Balancer "sticky sessions" (persistent cookies) to make it more clear which software package that handles this feature. The Splunk Enterprise Python SDK is included with the SOAR Splunk app to connect to a Splunk Enterprise Search Head (SH) server and/or Search Head Cluster (SHC). Using the proxy feature of the Burp Suite app to observe the behavior of the http requests & responses by the SOAR Splunk app via the Splunk Enterprise Python SDK to a Splunk Enterprise SHC with a F5 Load Balancer in front of it, the BIGipServer cookie used for persistence ("sticking sessions") is not included in subsequent http requests after receiving the BIGipServer cookie in the response of the first http request from the Splunk SOAR app. However as a comparison using a Firefox browser instead of the SOAR Splunk app as client to the Splunk Enterprise SHC, a Firefox browser does send the BIGipServer cookie in subsequent http requests after receiving the BIGipServer cookie in the response of the first http request from the Firefox browser. |
@Trek333 I agree the feature should be listed, and the unit tests should be updated with a test to avoid any future regressions. |
@bendikro we did include unit tests where if the Auth Cookie is not available, authentication is being done with username and password. Test cases were updated as part of this PR itself, would request you to refer them. Also there were multiple PRs for the fix of the issue #438 and the fix was delivered in 2 different releases. As for mentioning this feature it was mentioned in the CHANGELOG.md but under Bug Fix(version 1.6.20) and Minor Changes(version 1.7.0) - Reference. I hope I have answered your concern, please do let me know if anything I have missed. Thanks |
@ashah-splunk and @bendikro #485 pull request has been submitted to add a unit test that is specific to regression testing the persistence of 3rd party cookies by the Splunk Enterprise Python SDK. More specifically, this unit test is targeted at the use case of the Splunk Enterprise Python SDK supporting "sticky sessions" of a Load Balancer for a Splunk Enterprise SHC (Search Head Cluster). Please let me know your feedback and feel to free to add more unit tests for 3rd party cookie persistence. Thanks |
Changes in response to the issue #438